8311906: Improve robustness of String constructors with mutable array inputs #2437

GoeLin · 2025-10-31T14:33:19Z

I backport this for parity with 21.0.10-oracle.

I had to resolve and adapt the change to the CSR for 21, that differes from the CSR for 22.
In addition, I include four follow-up changes. All steps are seperated into individual commits.
In detail:

Resolved two files (first commit)

src/java.base/share/classes/java/lang/String.java

Resolved complex code in
private String(Charset charset, byte[] bytes, int offset, int length)
at line 563++

This is needed because later change
https://bugs.openjdk.org/browse/JDK-8320570: NegativeArraySizeException decoding >1G UTF8 bytes with non-ascii characters
was already backported.

In 22, this is removed:
- byte[] buf = new byte[length << 1];
In 21, it looks like this:
- byte[] buf = StringUTF16.newBytesFor(length);

I only updated variable buf to utf16 in that line effecitvely keeping 8320570.
After this change, the method looks the same as in 22.0.2.

src/java.base/share/classes/java/lang/StringUTF16.java

Resolved imports.

Adapted to CSR (second commit)

The CSRs for 21 and 22 differ. The CSR for 21 does not mention that the documentation is
changed. Thus, I omit corresponding edits, see extra commit that removes them. Similar
modifications to backports have been done before.

Follow-ups (commits 3-6)

The original change has some minor issues. Notable for example that the test StringRacyConstructor.java is failing. Three follow-up issues exist, and one recursive fix for a test. These are clean backports on top. I decided to include them here as working with four dependent PRs is quite cumbersome, and the main change needs review anyways. I guess reviewing a correct change has some value, too.
In case a backport to 17 is necessary, all can be grabbed from 21 together this way.

Adapt 8325590 to 21 (last commit)

The test modification of 8325590 is not compatible with Java 21. https://bugs.openjdk.org/browse/JDK-8310047: "Add UTF-32 based Charsets into StandardCharsets" is only in 22. Removed the corresponding test cases.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
JDK-8311906 needs maintainer approval
JDK-8321180 needs maintainer approval
Change requires CSR request JDK-8368232 to be approved
JDK-8321514 needs maintainer approval
JDK-8325590 needs maintainer approval
JDK-8322018 needs maintainer approval

Issues

JDK-8311906: Improve robustness of String constructors with mutable array inputs (Bug - P4 - Approved)
JDK-8321180: Condition for non-latin1 string size too large exception is off by one (Bug - P4 - Approved)
JDK-8322018: Test java/lang/String/CompactString/MaxSizeUTF16String.java fails with -Xcomp (Bug - P3 - Approved)
JDK-8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled (Bug - P2 - Approved)
JDK-8325590: Regression in round-tripping UTF-16 strings after JDK-8311906 (Bug - P1 - Approved)
JDK-8368232: Improve robustness of String constructors with mutable array inputs (CSR)

Reviewers

Ralf Schmelter (@schmelter-sap - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/2437/head:pull/2437
$ git checkout pull/2437

Update a local copy of the PR:
$ git checkout pull/2437
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/2437/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2437

View PR using the GUI difftool:
$ git pr show -t 2437

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/2437.diff

Using Webrev

Link to Webrev Comment

…e exception is off by one

…F16String.java fails with -Xcomp

…m codepoints if CompactStrings is not enabled

…fter JDK-8311906

bridgekeeper · 2025-10-31T14:35:23Z

👋 Welcome back goetz! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-10-31T14:36:22Z

@GoeLin This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8311906: Improve robustness of String constructors with mutable array inputs
8321180: Condition for non-latin1 string size too large exception is off by one
8322018: Test java/lang/String/CompactString/MaxSizeUTF16String.java fails with -Xcomp
8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled
8325590: Regression in round-tripping UTF-16 strings after JDK-8311906

Reviewed-by: rschmelter

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 17 new commits pushed to the master branch:

a04888a: 8316422: TestIntegerUnsignedDivMod.java triggers "invalid layout" assert in FrameValues::validate
946b81e: 8328299: Convert DnDFileGroupDescriptor.html applet test to main
dfd2ced: 8352686: Opensource JInternalFrame tests - series3
... and 14 more: https://git.openjdk.org/jdk21u-dev/compare/c5275d331cb82fb1ab6373236890691c3748e891...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-10-31T14:36:32Z

This backport pull request has now been updated with issue from the original commit.

GoeLin · 2025-10-31T15:19:24Z

/issue 8321180 8322018 8321514 8325590

openjdk · 2025-10-31T15:21:32Z

@GoeLin
Adding additional issue to issue list: 8321180: Condition for non-latin1 string size too large exception is off by one.

Adding additional issue to issue list: 8322018: Test java/lang/String/CompactString/MaxSizeUTF16String.java fails with -Xcomp.

Adding additional issue to issue list: 8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled.

Adding additional issue to issue list: 8325590: Regression in round-tripping UTF-16 strings after JDK-8311906.

GoeLin · 2025-10-31T17:31:24Z

GHA failure: test/jdk/java/util/concurrent/ExecutorService/CloseTest.java failed with
AssertionError: expected [true] but found [false]
We have seen this fail before, e.g. https://github.com/openjdk-bots/jdk21u-dev/actions/runs/17855005583/attempts/1#summary-50796380616 and I have recorded more failures. So I assume this failure is unrelated.

mlbridge · 2025-11-01T10:08:42Z

Webrevs

01: Full - Incremental (b80f2e0f)
00: Full (eeed7fa5)

schmelter-sap

Couldn't you just used Charset.forName("utf-32" / "utf-32be" / "utf-32le") in the ReadWriteString test instead of removing the test for these three encodings?

GoeLin · 2025-11-07T15:12:30Z

Couldn't you just used Charset.forName("utf-32" / "utf-32be" / "utf-32le") in the ReadWriteString test instead of removing the test for these three encodings?

That's a good point, yes that works. See new commit.

schmelter-sap

Looks good.

openjdk · 2025-11-09T10:23:13Z

⚠️ @GoeLin This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

GoeLin · 2025-11-10T13:50:11Z

/integrate

openjdk · 2025-11-10T13:51:21Z

Going to push as commit fbc9fad.
Since your change was applied there have been 18 commits pushed to the master branch:

be72cee: 8315990: Amend problemlisted tests to proper position
a04888a: 8316422: TestIntegerUnsignedDivMod.java triggers "invalid layout" assert in FrameValues::validate
946b81e: 8328299: Convert DnDFileGroupDescriptor.html applet test to main
... and 15 more: https://git.openjdk.org/jdk21u-dev/compare/c5275d331cb82fb1ab6373236890691c3748e891...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-11-10T13:51:29Z

@GoeLin Pushed as commit fbc9fad.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

GoeLin added 7 commits October 31, 2025 11:19

backport 155abc576a0212932825485380d4e2a9c7dd2fdc

8ef41ef

Modify comments according to diff in CSR for 22 and 21.

3c445e1

clean backport 8321180: Condition for non-latin1 string size too larg…

36dbe48

…e exception is off by one

clean backport 8322018: Test java/lang/String/CompactString/MaxSizeUT…

af06803

…F16String.java fails with -Xcomp

clean backport 8321514: UTF16 string gets constructed incorrectly fro…

891a667

…m codepoints if CompactStrings is not enabled

clean backport 8325590: Regression in round-tripping UTF-16 strings a…

982c750

…fter JDK-8311906

Adapt test in 8325590 to jdk21

eeed7fa

openjdk bot changed the title ~~backport 155abc576a0212932825485380d4e2a9c7dd2fdc~~ 8311906: Improve robustness of String constructors with mutable array inputs Oct 31, 2025

openjdk bot added the backport Port of a pull request already in a different code base label Oct 31, 2025

GoeLin marked this pull request as ready for review November 1, 2025 10:02

openjdk bot added rfr Pull request is ready for review approval Requires approval; will be removed when approval is received labels Nov 1, 2025

openjdk bot removed the approval Requires approval; will be removed when approval is received label Nov 1, 2025

schmelter-sap reviewed Nov 7, 2025

View reviewed changes

better solution for test fix

b80f2e0

schmelter-sap approved these changes Nov 9, 2025

View reviewed changes

openjdk bot added approval Requires approval; will be removed when approval is received ready Pull request is ready to be integrated and removed approval Requires approval; will be removed when approval is received labels Nov 9, 2025

openjdk bot added the integrated Pull request has been integrated label Nov 10, 2025

openjdk bot closed this Nov 10, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 10, 2025

GoeLin deleted the goetz_backport_8311906 branch November 10, 2025 13:52

8311906: Improve robustness of String constructors with mutable array inputs #2437

8311906: Improve robustness of String constructors with mutable array inputs #2437

Uh oh!

Conversation

GoeLin commented Oct 31, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Oct 31, 2025

Uh oh!

openjdk bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Oct 31, 2025

Uh oh!

GoeLin commented Oct 31, 2025

Uh oh!

openjdk bot commented Oct 31, 2025

Uh oh!

GoeLin commented Oct 31, 2025

Uh oh!

mlbridge bot commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

schmelter-sap left a comment

Choose a reason for hiding this comment

Uh oh!

GoeLin commented Nov 7, 2025

Uh oh!

schmelter-sap left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Nov 9, 2025

Uh oh!

GoeLin commented Nov 10, 2025

Uh oh!

openjdk bot commented Nov 10, 2025

Uh oh!

openjdk bot commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

GoeLin commented Oct 31, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 31, 2025 •

edited

Loading

mlbridge bot commented Nov 1, 2025 •

edited

Loading